From 1f47c9b9aecd590062e097820f109faa1524c1c7 Mon Sep 17 00:00:00 2001 From: btongminh Date: Sun, 17 Nov 2013 18:52:54 +0100 Subject: [PATCH] Fix capitalization in ApiQueryBase::titlePartToKey() ApiQueryBase::titlePartToKey now allows an extra parameter that indicates the namespace in order to properly capitalize the title part. This allows list=allcategories, list=allimages, list=alllinks, list=allpages, list=deletedrevs and list=filearchive to handle case-sensitivity properly for all parameters. Bug: 25702 Change-Id: Iaa5a71ec536f3716f54bc84b39f645545dfd8660 --- RELEASE-NOTES-1.23 | 5 ++ includes/api/ApiQueryAllCategories.php | 9 +-- includes/api/ApiQueryAllImages.php | 9 +-- includes/api/ApiQueryAllLinks.php | 13 +++-- includes/api/ApiQueryAllPages.php | 9 +-- includes/api/ApiQueryBase.php | 22 +++++++- includes/api/ApiQueryDeletedrevs.php | 9 +-- includes/api/ApiQueryFilearchive.php | 9 +-- tests/TestsAutoLoader.php | 1 + .../includes/api/ApiQueryAllPagesTest.php | 30 ++++++++++ .../phpunit/includes/api/MockApiQueryBase.php | 11 ++++ .../includes/api/query/ApiQueryTest.php | 56 +++++++++++++++++++ 12 files changed, 154 insertions(+), 29 deletions(-) create mode 100644 tests/phpunit/includes/api/ApiQueryAllPagesTest.php create mode 100644 tests/phpunit/includes/api/MockApiQueryBase.php diff --git a/RELEASE-NOTES-1.23 b/RELEASE-NOTES-1.23 index 3e40f7fc6e..416a7fc936 100644 --- a/RELEASE-NOTES-1.23 +++ b/RELEASE-NOTES-1.23 @@ -85,6 +85,11 @@ production. MediaWiki 1.24. * action=parse now has disabletoc flag to disable table of contents in output. * SpecialRecentChanges::feedSetup() was removed. +* (bug 25702) list=allcategories, list=allimages, list=alllinks, list=allpages, + list=deletedrevs and list=filearchive did not handle case-sensitivity + properly for all parameters. +* ApiQueryBase::titlePartToKey allows an extra parameter that indicates the + namespace in order to properly capitalize the title part. === Languages updated in 1.23 === diff --git a/includes/api/ApiQueryAllCategories.php b/includes/api/ApiQueryAllCategories.php index d0ab59e98c..6bf8075508 100644 --- a/includes/api/ApiQueryAllCategories.php +++ b/includes/api/ApiQueryAllCategories.php @@ -67,8 +67,8 @@ class ApiQueryAllCategories extends ApiQueryGeneratorBase { } $dir = ( $params['dir'] == 'descending' ? 'older' : 'newer' ); - $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) ); - $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) ); + $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], NS_CATEGORY ) ); + $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], NS_CATEGORY ) ); $this->addWhereRange( 'cat_title', $dir, $from, $to ); $min = $params['min']; @@ -80,8 +80,9 @@ class ApiQueryAllCategories extends ApiQueryGeneratorBase { } if ( isset( $params['prefix'] ) ) { - $this->addWhere( 'cat_title' . - $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) ); + $this->addWhere( 'cat_title' . $db->buildLike( + $this->titlePartToKey( $params['prefix'], NS_CATEGORY ), + $db->anyString() ) ); } $this->addOption( 'LIMIT', $params['limit'] + 1 ); diff --git a/includes/api/ApiQueryAllImages.php b/includes/api/ApiQueryAllImages.php index 9f97cac9ed..4095bd81fd 100644 --- a/includes/api/ApiQueryAllImages.php +++ b/includes/api/ApiQueryAllImages.php @@ -132,13 +132,14 @@ class ApiQueryAllImages extends ApiQueryGeneratorBase { } // Image filters - $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) ); - $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) ); + $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], NS_FILE ) ); + $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], NS_FILE ) ); $this->addWhereRange( 'img_name', ( $ascendingOrder ? 'newer' : 'older' ), $from, $to ); if ( isset( $params['prefix'] ) ) { - $this->addWhere( 'img_name' . - $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) ); + $this->addWhere( 'img_name' . $db->buildLike( + $this->titlePartToKey( $params['prefix'], NS_FILE ), + $db->anyString() ) ); } } else { // Check mutually exclusive params diff --git a/includes/api/ApiQueryAllLinks.php b/includes/api/ApiQueryAllLinks.php index ff53d0f46a..5be304d34c 100644 --- a/includes/api/ApiQueryAllLinks.php +++ b/includes/api/ApiQueryAllLinks.php @@ -148,15 +148,16 @@ class ApiQueryAllLinks extends ApiQueryGeneratorBase { } // 'continue' always overrides 'from' - $from = $continue || is_null( $params['from'] ) - ? null - : $this->titlePartToKey( $params['from'] ); - $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) ); + $from = ( $continue || $params['from'] === null ? null : + $this->titlePartToKey( $params['from'], $params['namespace'] ) ); + $to = ( $params['to'] === null ? null : + $this->titlePartToKey( $params['to'], $params['namespace'] ) ); $this->addWhereRange( $pfx . $fieldTitle, 'newer', $from, $to ); + if ( isset( $params['prefix'] ) ) { - $this->addWhere( $pfx . $fieldTitle . - $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) ); + $this->addWhere( $pfx . $fieldTitle . $db->buildLike( $this->titlePartToKey( + $params['prefix'], $params['namespace'] ), $db->anyString() ) ); } $this->addFields( array( 'pl_title' => $pfx . $fieldTitle ) ); diff --git a/includes/api/ApiQueryAllPages.php b/includes/api/ApiQueryAllPages.php index 363d6578f2..430dd51716 100644 --- a/includes/api/ApiQueryAllPages.php +++ b/includes/api/ApiQueryAllPages.php @@ -87,13 +87,14 @@ class ApiQueryAllPages extends ApiQueryGeneratorBase { $this->addWhereFld( 'page_namespace', $params['namespace'] ); $dir = ( $params['dir'] == 'descending' ? 'older' : 'newer' ); - $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) ); - $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) ); + $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], $params['namespace'] ) ); + $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], $params['namespace'] ) ); $this->addWhereRange( 'page_title', $dir, $from, $to ); if ( isset( $params['prefix'] ) ) { - $this->addWhere( 'page_title' . - $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) ); + $this->addWhere( 'page_title' . $db->buildLike( + $this->titlePartToKey( $params['prefix'], $params['namespace'] ), + $db->anyString() ) ); } if ( is_null( $resultPageSet ) ) { diff --git a/includes/api/ApiQueryBase.php b/includes/api/ApiQueryBase.php index fcd318027d..d9aacaaf5d 100644 --- a/includes/api/ApiQueryBase.php +++ b/includes/api/ApiQueryBase.php @@ -480,12 +480,28 @@ abstract class ApiQueryBase extends ApiBase { } /** - * An alternative to titleToKey() that doesn't trim trailing spaces + * An alternative to titleToKey() that doesn't trim trailing spaces, and + * does not mangle the input if starts with something that looks like a + * namespace. It is advisable to pass the namespace parameter in order to + * handle per-namespace capitalization settings. * @param string $titlePart Title part with spaces + * @param $defaultNamespace int Namespace to assume * @return string Title part with underscores */ - public function titlePartToKey( $titlePart ) { - return substr( $this->titleToKey( $titlePart . 'x' ), 0, -1 ); + public function titlePartToKey( $titlePart, $defaultNamespace = NS_MAIN ) { + $t = Title::makeTitleSafe( $defaultNamespace, $titlePart . 'x' ); + if ( !$t ) { + $this->dieUsageMsg( array( 'invalidtitle', $titlePart ) ); + } + if ( $defaultNamespace != $t->getNamespace() || $t->getInterwiki() !== '' ) { + // This can happen in two cases. First, if you call titlePartToKey with a title part + // that looks like a namespace, but with $defaultNamespace = NS_MAIN. It would be very + // difficult to handle such a case. Such cases cannot exist and are therefore treated + // as invalid user input. The second case is when somebody specifies a title interwiki + // prefix. + $this->dieUsageMsg( array( 'invalidtitle', $titlePart ) ); + } + return substr( $t->getDbKey(), 0, -1 ); } /** diff --git a/includes/api/ApiQueryDeletedrevs.php b/includes/api/ApiQueryDeletedrevs.php index f63e0330e9..4e4b2cce23 100644 --- a/includes/api/ApiQueryDeletedrevs.php +++ b/includes/api/ApiQueryDeletedrevs.php @@ -170,13 +170,14 @@ class ApiQueryDeletedrevs extends ApiQueryBase { } elseif ( $mode == 'all' ) { $this->addWhereFld( 'ar_namespace', $params['namespace'] ); - $from = is_null( $params['from'] ) ? null : $this->titleToKey( $params['from'] ); - $to = is_null( $params['to'] ) ? null : $this->titleToKey( $params['to'] ); + $from = $params['from'] === null ? null : $this->titlePartToKey( $params['from'], $params['namespace'] ); + $to = $params['to'] === null ? null : $this->titlePartToKey( $params['to'], $params['namespace'] ); $this->addWhereRange( 'ar_title', $dir, $from, $to ); if ( isset( $params['prefix'] ) ) { - $this->addWhere( 'ar_title' . - $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) ); + $this->addWhere( 'ar_title' . $db->buildLike( + $this->titlePartToKey( $params['prefix'], $params['namespace'] ), + $db->anyString() ) ); } } diff --git a/includes/api/ApiQueryFilearchive.php b/includes/api/ApiQueryFilearchive.php index f8f455800e..e2c9964379 100644 --- a/includes/api/ApiQueryFilearchive.php +++ b/includes/api/ApiQueryFilearchive.php @@ -88,15 +88,16 @@ class ApiQueryFilearchive extends ApiQueryBase { // Image filters $dir = ( $params['dir'] == 'descending' ? 'older' : 'newer' ); - $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) ); + $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], NS_FILE ) ); if ( !is_null( $params['continue'] ) ) { $from = $params['continue']; } - $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) ); + $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], NS_FILE ) ); $this->addWhereRange( 'fa_name', $dir, $from, $to ); if ( isset( $params['prefix'] ) ) { - $this->addWhere( 'fa_name' . - $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) ); + $this->addWhere( 'fa_name' . $db->buildLike( + $this->titlePartToKey( $params['prefix'], NS_FILE ), + $db->anyString() ) ); } $sha1Set = isset( $params['sha1'] ); diff --git a/tests/TestsAutoLoader.php b/tests/TestsAutoLoader.php index 46a894cea9..58df552a97 100644 --- a/tests/TestsAutoLoader.php +++ b/tests/TestsAutoLoader.php @@ -57,6 +57,7 @@ $wgAutoloadClasses += array( 'ApiTestCase' => "$testDir/phpunit/includes/api/ApiTestCase.php", 'ApiTestContext' => "$testDir/phpunit/includes/api/ApiTestContext.php", 'MockApi' => "$testDir/phpunit/includes/api/MockApi.php", + 'MockApiQueryBase' => "$testDir/phpunit/includes/api/MockApiQueryBase.php", 'UserWrapper' => "$testDir/phpunit/includes/api/UserWrapper.php", 'RandomImageGenerator' => "$testDir/phpunit/includes/api/RandomImageGenerator.php", diff --git a/tests/phpunit/includes/api/ApiQueryAllPagesTest.php b/tests/phpunit/includes/api/ApiQueryAllPagesTest.php new file mode 100644 index 0000000000..bc08afe066 --- /dev/null +++ b/tests/phpunit/includes/api/ApiQueryAllPagesTest.php @@ -0,0 +1,30 @@ +doLogin(); + } + + function testBug25702() { + $title = Title::newFromText( 'Category:Template:xyz' ); + $page = WikiPage::factory( $title ); + $page->doEdit( 'Some text', 'inserting content' ); + + $result = $this->doApiRequest( array( + 'action' => 'query', + 'list' => 'allpages', + 'apnamespace' => NS_CATEGORY, + 'apprefix' => 'Template:x' ) ); + + $this->assertArrayHasKey( 'query', $result[0] ); + $this->assertArrayHasKey( 'allpages', $result[0]['query'] ); + $this->assertNotEquals( 0, count( $result[0]['query']['allpages'] ), + 'allpages list does not contain page Category:Template:xyz' ); + } +} diff --git a/tests/phpunit/includes/api/MockApiQueryBase.php b/tests/phpunit/includes/api/MockApiQueryBase.php new file mode 100644 index 0000000000..4bede51915 --- /dev/null +++ b/tests/phpunit/includes/api/MockApiQueryBase.php @@ -0,0 +1,11 @@ +doLogin(); + + // Setup en: as interwiki prefix + $this->hooks = $wgHooks; + $wgHooks['InterwikiLoadPrefix'][] = function ( $prefix, &$data ) { + if ( $prefix == 'en' ) { + $data = array( 'iw_url' => 'wikipedia' ); + } + return false; + }; + } + + protected function tearDown() { + global $wgHooks; + $wgHooks = $this->hooks; + + parent::tearDown(); } public function testTitlesGetNormalized() { @@ -64,4 +86,38 @@ class ApiQueryTest extends ApiTestCase { $this->assertArrayHasKey( 'missing', $data[0]['query']['pages'][-2] ); $this->assertArrayHasKey( 'invalid', $data[0]['query']['pages'][-1] ); } + + /** + * Test the ApiBase::titlePartToKey function + * + * @param string $titlePart + * @param int $namespace + * @param string $expected + * @param string $description + * @dataProvider provideTestTitlePartToKey + */ + function testTitlePartToKey( $titlePart, $namespace, $expected, $expectException ) { + $api = new MockApiQueryBase(); + $exceptionCaught = false; + try { + $this->assertEquals( $expected, $api->titlePartToKey( $titlePart, $namespace ) ); + } catch ( UsageException $e ) { + $exceptionCaught = true; + } + $this->assertEquals( $expectException, $exceptionCaught, + 'UsageException thrown by titlePartToKey' ); + } + + function provideTestTitlePartToKey() { + return array( + array( 'a b c', NS_MAIN, 'A_b_c', false ), + array( 'x', NS_MAIN, 'X', false ), + array( 'y ', NS_MAIN, 'Y_', false ), + array( 'template:foo', NS_CATEGORY, 'Template:foo', false ), + array( 'en:foo', NS_CATEGORY, 'En:foo', false ), + array( "\xF7", NS_MAIN, null, true ), + array( 'template:foo', NS_MAIN, null, true ), + array( 'en:foo', NS_MAIN, null, true ), + ); + } } -- 2.20.1